Skip to content

[libc] provide binary compatibility with syscall symbol #117229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
SchrodingerZhu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[libc] provide binary compatibility with syscall symbol #117229

SchrodingerZhu wants to merge 1 commit into from

Conversation

@SchrodingerZhu
Copy link
Contributor

@SchrodingerZhu SchrodingerZhu commented Nov 21, 2024 •
edited
Loading

Glibc and MUSL both provides syscall as a binary. Inside LLVM-libc, we provides it as a macro backed by __llvm_libc_syscall which is not binary compatible with other implementations. This PR provides the symbol for better compatibility for those applications assuming the existence of the symbol.

@llvmbot llvmbot added the libc label Nov 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2024

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/117229.diff

9 Files Affected:

  • (modified) libc/config/linux/aarch64/entrypoints.txt (+1)
  • (modified) libc/config/linux/riscv/entrypoints.txt (+1)
  • (modified) libc/config/linux/x86_64/entrypoints.txt (+1)
  • (modified) libc/src/unistd/CMakeLists.txt (+7)
  • (modified) libc/src/unistd/linux/CMakeLists.txt (+10)
  • (added) libc/src/unistd/linux/syscall_compat.cpp (+32)
  • (added) libc/src/unistd/syscall_compat.h (+21)
  • (modified) libc/test/src/unistd/CMakeLists.txt (+1)
  • (modified) libc/test/src/unistd/syscall_test.cpp (+10)
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 74ca3742977a5f..9a34029ab78230 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -1002,6 +1002,7 @@ if(LLVM_LIBC_FULL_BUILD)
 
     # unistd.h entrypoints
     libc.src.unistd.__llvm_libc_syscall
+    libc.src.unistd.syscall_compat
     libc.src.unistd._exit
     libc.src.unistd.environ
     libc.src.unistd.execv
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 5419462d4f5b3b..d7bdc6daa1b01c 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -940,6 +940,7 @@ if(LLVM_LIBC_FULL_BUILD)
 
     # unistd.h entrypoints
     libc.src.unistd.__llvm_libc_syscall
+    libc.src.unistd.syscall_compat
     libc.src.unistd._exit
     libc.src.unistd.environ
     libc.src.unistd.execv
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 957e28bd66cc4c..5b6aa83712c991 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -1094,6 +1094,7 @@ if(LLVM_LIBC_FULL_BUILD)
 
     # unistd.h entrypoints
     libc.src.unistd.__llvm_libc_syscall
+    libc.src.unistd.syscall_compat
     libc.src.unistd._exit
     libc.src.unistd.environ
     libc.src.unistd.execv
diff --git a/libc/src/unistd/CMakeLists.txt b/libc/src/unistd/CMakeLists.txt
index 1a0b2e3293d03c..29f71ceca64750 100644
--- a/libc/src/unistd/CMakeLists.txt
+++ b/libc/src/unistd/CMakeLists.txt
@@ -252,6 +252,13 @@ add_entrypoint_object(
     .${LIBC_TARGET_OS}.__llvm_libc_syscall
 )
 
+add_entrypoint_object(
+  syscall_compat
+  ALIAS
+  DEPENDS
+    .${LIBC_TARGET_OS}.syscall_compat
+)
+
 add_entrypoint_object(
   sysconf
   ALIAS
diff --git a/libc/src/unistd/linux/CMakeLists.txt b/libc/src/unistd/linux/CMakeLists.txt
index 8a448731414143..106477cec2b07d 100644
--- a/libc/src/unistd/linux/CMakeLists.txt
+++ b/libc/src/unistd/linux/CMakeLists.txt
@@ -465,6 +465,16 @@ add_entrypoint_object(
     libc.src.errno.errno
 )
 
+add_entrypoint_object(
+  syscall_compat
+  SRCS
+  syscall_compat.cpp
+  HDRS
+    ../syscall_compat.h
+  DEPENDS
+    .__llvm_libc_syscall
+)
+
 add_entrypoint_object(
   sysconf
   SRCS
diff --git a/libc/src/unistd/linux/syscall_compat.cpp b/libc/src/unistd/linux/syscall_compat.cpp
new file mode 100644
index 00000000000000..fc0039e15552f7
--- /dev/null
+++ b/libc/src/unistd/linux/syscall_compat.cpp
@@ -0,0 +1,32 @@
+//===-- Linux implementation of syscall (va compat) -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/unistd/syscall_compat.h"
+#include "src/__support/common.h"
+#include "src/unistd/syscall.h"
+
+#include "src/__support/macros/config.h"
+#include <stdarg.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+#undef syscall
+LLVM_LIBC_FUNCTION(long, syscall, (long n, ...)) {
+  va_list args;
+  va_start(args, n);
+  long arg1 = va_arg(args, long);
+  long arg2 = va_arg(args, long);
+  long arg3 = va_arg(args, long);
+  long arg4 = va_arg(args, long);
+  long arg5 = va_arg(args, long);
+  long arg6 = va_arg(args, long);
+  va_end(args);
+  return __llvm_libc_syscall(n, arg1, arg2, arg3, arg4, arg5, arg6);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/unistd/syscall_compat.h b/libc/src/unistd/syscall_compat.h
new file mode 100644
index 00000000000000..4f2271ce52d139
--- /dev/null
+++ b/libc/src/unistd/syscall_compat.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for syscall (va compat) -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_UNISTD_SYSCALL_COMPAT_H
+#define LLVM_LIBC_SRC_UNISTD_SYSCALL_COMPAT_H
+
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+#pragma push_macro("syscall")
+#undef syscall
+long syscall(long number, ...);
+#pragma pop_macro("syscall")
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_UNISTD_SYSCALL_COMPAT_H
diff --git a/libc/test/src/unistd/CMakeLists.txt b/libc/test/src/unistd/CMakeLists.txt
index e036e09cde702e..43491202626a1f 100644
--- a/libc/test/src/unistd/CMakeLists.txt
+++ b/libc/test/src/unistd/CMakeLists.txt
@@ -429,6 +429,7 @@ add_libc_unittest(
     libc.include.sys_syscall
     libc.src.errno.errno 
     libc.src.unistd.__llvm_libc_syscall
+    libc.src.unistd.syscall_compat
     libc.test.UnitTest.ErrnoSetterMatcher
 )
 
diff --git a/libc/test/src/unistd/syscall_test.cpp b/libc/test/src/unistd/syscall_test.cpp
index f6cc3eab9aabe8..9ac842267b4047 100644
--- a/libc/test/src/unistd/syscall_test.cpp
+++ b/libc/test/src/unistd/syscall_test.cpp
@@ -8,6 +8,7 @@
 
 #include "src/errno/libc_errno.h"
 #include "src/unistd/syscall.h"
+#include "src/unistd/syscall_compat.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
@@ -177,3 +178,12 @@ TEST(LlvmLibcSyscallTest, FileLinkCreateDestroy) {
   ASSERT_GE(LIBC_NAMESPACE::syscall(SYS_close, dir_fd), 0l);
   ASSERT_ERRNO_SUCCESS();
 }
+
+TEST(LlvmLibcSyscallTest, TrivialCallCompat) {
+  LIBC_NAMESPACE::libc_errno = 0;
+#pragma push_macro("syscall")
+#undef syscall
+  ASSERT_GE(LIBC_NAMESPACE::syscall(SYS_gettid), 0l);
+#pragma pop_macro("syscall")
+  ASSERT_ERRNO_SUCCESS();
+}

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Nov 21, 2024

Mind adding a description to this PR?

@SchrodingerZhu
Copy link
Contributor Author

SchrodingerZhu commented Nov 21, 2024

Ah, sorry I missed it when committing.

@michaelrj-google
Copy link
Contributor

michaelrj-google commented Nov 21, 2024

I'm not sure why this is necessary. We already have an implementation of syscall: https://github.com/llvm/llvm-project/blob/main/libc/src/unistd/linux/syscall.cpp

It's got a different name that's translated through a macro in the unistd header: https://github.com/llvm/llvm-project/blob/main/libc/include/llvm-libc-macros/linux/unistd-macros.h#L52
This means we don't need to worry about it being variadic, it always takes 6 arguments, with the rest filled in by the macro.

@SchrodingerZhu
Copy link
Contributor Author

SchrodingerZhu commented Nov 21, 2024

For example, rust std assumes syscall.

          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.02.rcgu.o): in function `std::backtrace_rs::symbolize::gimli::mmap::Mmap::map':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/../../backtrace/src/symbolize/gimli/mmap_unix.rs:19:(.text._ZN3std12backtrace_rs9symbolize5gimli4mmap4Mmap3map17hc94ca9a7d9666f3aE+0x60): undefined reference to `mmap64'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.04.rcgu.o): in function `std::backtrace_rs::symbolize::gimli::libs_dl_iterate_phdr::native_libraries':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/../../backtrace/src/symbolize/gimli/libs_dl_iterate_phdr.rs:15:(.text._ZN3std12backtrace_rs9symbolize5gimli20libs_dl_iterate_phdr16native_libraries17h0b64ac80f38a824cE+0x38): undefined reference to `dl_iterate_phdr'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::File::open_c::{{closure}}':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fs.rs:1185:(.text._ZN3std3sys3pal4unix2fs4File6open_c28_$u7b$$u7b$closure$u7d$$u7d$17h84c4370f703adb74E+0x35): undefined reference to `open64'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::File::file_attr':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fs.rs:1204:(.text._ZN3std3sys3pal4unix2fs4File9file_attr17hf80f8292d2be1bf4E+0x103): undefined reference to `fstat64'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::File::seek':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fs.rs:1439:(.text._ZN3std3sys3pal4unix2fs4File4seek17heeecd2aca42ba244E+0xc0): undefined reference to `lseek64'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::stat::{{closure}}':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fs.rs:1894:(.text._ZN3std3sys3pal4unix2fs4stat28_$u7b$$u7b$closure$u7d$$u7d$17h88326fd6fbc9bad1E+0x10f): undefined reference to `stat64'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::canonicalize::{{closure}}':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fs.rs:1920:(.text._ZN3std3sys3pal4unix2fs12canonicalize28_$u7b$$u7b$closure$u7d$$u7d$17hbf4ec9940e9a0e09E+0x50): undefined reference to `realpath'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::futex::futex_wait':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/futex.rs:72:(.text._ZN3std3sys3pal4unix5futex10futex_wait17h55857f1517ee301aE+0xe1): undefined reference to `syscall'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::futex::futex_wake':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/futex.rs:105:(.text._ZN3std3sys3pal4unix5futex10futex_wake17heba2048a7f0d47ccE+0x2a): undefined reference to `syscall'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::futex::futex_wake_all':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/futex.rs:114:(.text._ZN3std3sys3pal4unix5futex14futex_wake_all17hd38aa3f913171797E+0x2a): undefined reference to `syscall'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.05.rcgu.o): in function `std::sys::pal::unix::fs::try_statx::statx':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/weak.rs:171:(.text._ZN3std3sys3pal4unix2fs9try_statx5statx17ha3e238ae5b888528E+0xbb): undefined reference to `syscall'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.08.rcgu.o): in function `std::sys::pal::unix::os::errno':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/os.rs:67:(.text._ZN3std3sys3pal4unix2os5errno17h0682667f97f534e6E+0x2): undefined reference to `__errno_location'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.08.rcgu.o): in function `std::sys::pal::unix::os::error_string':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/os.rs:133:(.text._ZN3std3sys3pal4unix2os12error_string17h5c358e28d83321d3E+0x62): undefined reference to `__xpg_strerror_r'
          /usr/bin/ld: /home/schrodingerzy/Documents/rust-linux-llvm/target/x86_64-llvm-linux-gnu/debug/deps/libstd-29cb47d2f4e02d7c.rlib(std-29cb47d2f4e02d7c.std.7aa8159242a71b71-cgu.14.rcgu.o): in function `std::sys::pal::unix::fd::FileDesc::write_vectored':
          /home/schrodingerzy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/pal/unix/fd.rs:323:(.text._ZN3std3sys3pal4unix2fd8FileDesc14write_vectored17h7e90fe10701ce2a7E+0x58): undefined reference to `writev'

@SchrodingerZhu
Copy link
Contributor Author

SchrodingerZhu commented Nov 21, 2024 •
edited
Loading

I'm not sure why this is necessary. We already have an implementation of syscall: https://github.com/llvm/llvm-project/blob/main/libc/src/unistd/linux/syscall.cpp

It's got a different name that's translated through a macro in the unistd header: https://github.com/llvm/llvm-project/blob/main/libc/include/llvm-libc-macros/linux/unistd-macros.h#L52 This means we don't need to worry about it being variadic, it always takes 6 arguments, with the rest filled in by the macro.

I think the problem is that even though we do not assume ABI compatibility with glibc compiled programs, in current approach, we are not providing standard means to programs outside C/C++ to do platform-agnostic syscall.

https://github.com/rust-lang/rust/blob/b19329a37cedf2027517ae22c87cf201f93d776e/library/std/src/sys/pal/unix/linux/pidfd.rs#L17

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Nov 21, 2024

IIUC, it looks like the rust std lib is expecting to use C FFI to do syscalls:

https://codebrowser.dev/rust/rust/library/std/src/sys/pal/unix/weak.rs.html#158

IIUC, it doesn't matter if our syscall is a macro, if the runtime of the higher level language expects syscall to be a symbol and can't use the C preprocessor (such as rust).

@SchrodingerZhu
Copy link
Contributor Author

SchrodingerZhu commented Nov 21, 2024 •
edited
Loading

@nickdesaulniers yes, but maybe we should ask them to change instead if they are to support LLVM's libc? I am open for discussion.

e.g. they can use __llvm_libc_syscall instead, but it may seems a bit ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

libc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SchrodingerZhu @llvmbot @nickdesaulniers @michaelrj-google